-
Notifications
You must be signed in to change notification settings - Fork 13.5k
Distinguish delim kind to decide whether to emit unexpected closing delimiter #138554
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
rustbot has assigned @compiler-errors. Use |
r? compiler |
I changed this part last year, will review this PR later. r? @chenyukang |
tests/ui/parser/issues/issue-67377-invalid-syntax-in-enum-discriminant.stderr
Outdated
Show resolved
Hide resolved
I think this solution introduces some kind of regression on other scenarios(such as the cases I commented). Maybe we should consider this as a special corner case and provide a special diagnostic for it, it is the pattern that one delimiter is missing the openning delimiter(it's in the inner part), except for this everything is ok, so we should suggest something like "missing openning delimiter ....". such as:
|
@rustbot author |
Reminder, once the PR becomes ready for a review, use |
Thanks, I will revise it soon! |
This seems like quite a lot of changes, I'll comment in the code to make it easier to REVIEW. In the first commit, I added the test. In the second commit I did some clean. I changed a variable name, the original name was inaccurate, I used In the third commit, I make a distinction between the different delimiters before triggering a |
I've combined them into one line so it's more aesthetically pleasing.
Since the |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This time, I reserve the mismatched
error when the situation is like {[)}
.
And I remove the mismatched
error only when close_delim_err
, which means double close delims.
Meanwhile, I remove duplicate missing open ... for this delimiter
in this file contains an unclosed delimiter
error because mismatched
errors have already emitted it.
tests/ui/parser/issues/issue-67377-invalid-syntax-in-enum-discriminant.stderr
Show resolved
Hide resolved
For this comment #138554 (comment) Unfortually, As to why the |
got it, thanks for the explanation. |
I don't have other questions except for #138554 (comment) |
This commit 0682e9e is for #138554 (comment). I use The last commit a1d2557 is for comment #138554 (comment). In all the cases, we should emit
Now, all the changes is
That's all we need. But the code needs some clean. If everything is Ok, I will clean the code and squash commits. 😃 |
The last commit is necessary if one is in a conservative consideration, as #138401 seems to be just to address cases like, |
I think we can remove those hint, since we reported mismatch above. |
Ok, I'll revise it later. |
But I have a concern that we only reported what went wrong in the mismatched error, but we didn't mention how to correct it. I think this hint complements this. If you think it should be removed, I will remove the last commit later. |
for a mismatched scenario, the hint is not complementing it:
perfer to remove it. |
Signed-off-by: xizheyin <[email protected]>
Make sense! I removed it. And I cleaned the code, and squashed them to 2 commits. Now, it should be what we want! |
Thanks! @bors r=chenyukang |
Thank you for your patient guidance! :) |
Distinguish delim kind to decide whether to emit unexpected closing delimiter Fixes rust-lang#138401
Distinguish delim kind to decide whether to emit unexpected closing delimiter Fixes rust-lang#138401
Rollup of 9 pull requests Successful merges: - #138554 (Distinguish delim kind to decide whether to emit unexpected closing delimiter) - #142673 (Show the offset, length and memory of uninit read errors) - #142693 (More robustly deal with relaxed bounds and improve their diagnostics) - #143382 (stabilize `const_slice_reverse`) - #143928 (opt-dist: make llvm builds optional) - #143961 (Correct which exploit mitigations are enabled by default) - #144050 (Fix encoding of link_section and no_mangle cross crate) - #144059 (Refactor `CrateLoader` into the `CStore`) - #144123 (Generalize `unsize` and `unsize_into` destinations) r? `@ghost` `@rustbot` modify labels: rollup
Fixes #138401